Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(k8s): if the output is json, stringify it #1728

Merged
merged 3 commits into from
Mar 25, 2020

Conversation

ThePixelDeveloper
Copy link
Contributor

@ThePixelDeveloper ThePixelDeveloper commented Mar 24, 2020

What this PR does / why we need it:

I spotted this issue with a task that ran the AWS CLI to create DynamoDB tables. The output of the command is in JSON. When the output is JSON it comes into garden as an object, which doesn't have a trim method. You can see this in run.ts.

You can recreate this issue by having a task output JSON, ala.

#!/usr/bin/env bash

# Forced for the sake of the test.
echo '{"hello": "world"}'
exit 0

setup.sh

kind: Module
name: database-tables
description: Create database tables
type: container
tasks:
 - name: db-migrate-default
   command: ["bash", "/project/setup.sh"]
   dependencies:
     - database-default

garden.yml

and then a docker file that just copies over the setup.sh file.

Which issue(s) this PR fixes:

Internal issue at the company I work for

Special notes for your reviewer:

@edvald
Copy link
Collaborator

edvald commented Mar 24, 2020

Thanks @ThePixelDeveloper! That's odd. It appears the Kubernetes API library does something implicitly there. I'm wondering why and how exactly though. Can you confirm that the log looks exactly as expected, with your fix?

@ThePixelDeveloper
Copy link
Contributor Author

ThePixelDeveloper commented Mar 24, 2020

@edvald Yeah I've confirmed this from stepping through the code in Google Chrome. I've run the task manually with a higher log level and confirmed it all looks good 👍

@edvald edvald merged commit d62890d into garden-io:master Mar 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants